Skip to content

fix(archive): skip checkpoint when worktree was deleted externally#1910

Merged
jonathanlab merged 1 commit intomainfrom
04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally
Apr 29, 2026
Merged

fix(archive): skip checkpoint when worktree was deleted externally#1910
jonathanlab merged 1 commit intomainfrom
04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally

Conversation

@jonathanlab
Copy link
Copy Markdown
Contributor

@jonathanlab jonathanlab commented Apr 28, 2026

Closes #1798

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 28, 2026

Prompt To Fix All With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/archive/service.ts
Line: 187-218

Comment:
**Prefer `if/else` over two separate `if` blocks**

The two independent `if (!worktreeIsValid)` / `if (worktreeIsValid)` blocks are mutually exclusive and can be collapsed into a single `if/else`, removing the superfluous second condition check and making the intent clearer.

```suggestion
        if (worktreeIsValid) {
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "fix(archive): skip checkpoint when workt..." | Re-trigger Greptile

Comment on lines +187 to +218
if (!worktreeIsValid) {
log.warn(
`Worktree at ${worktreePath} is missing or not a git repository; skipping checkpoint capture`,
);
archivedTask.checkpointId = null;
}

if (worktreeIsValid) {
const actualBranch = await this.getCurrentBranchName(worktreePath);
if (actualBranch && actualBranch !== "HEAD") {
archivedTask.branchName = actualBranch;
}

await step(
async () => {
if (!archivedTask.checkpointId) {
throw new Error("checkpointId must be set for worktree mode");
}
await this.captureWorktreeCheckpoint(
folderPath,
worktreePath,
archivedTask.checkpointId,
);
},
async () => {
if (archivedTask.checkpointId) {
const git = createGitClient(folderPath);
await deleteCheckpoint(git, archivedTask.checkpointId);
}
},
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Prefer if/else over two separate if blocks

The two independent if (!worktreeIsValid) / if (worktreeIsValid) blocks are mutually exclusive and can be collapsed into a single if/else, removing the superfluous second condition check and making the intent clearer.

Suggested change
if (!worktreeIsValid) {
log.warn(
`Worktree at ${worktreePath} is missing or not a git repository; skipping checkpoint capture`,
);
archivedTask.checkpointId = null;
}
if (worktreeIsValid) {
const actualBranch = await this.getCurrentBranchName(worktreePath);
if (actualBranch && actualBranch !== "HEAD") {
archivedTask.branchName = actualBranch;
}
await step(
async () => {
if (!archivedTask.checkpointId) {
throw new Error("checkpointId must be set for worktree mode");
}
await this.captureWorktreeCheckpoint(
folderPath,
worktreePath,
archivedTask.checkpointId,
);
},
async () => {
if (archivedTask.checkpointId) {
const git = createGitClient(folderPath);
await deleteCheckpoint(git, archivedTask.checkpointId);
}
},
);
}
if (worktreeIsValid) {
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/code/src/main/services/archive/service.ts
Line: 187-218

Comment:
**Prefer `if/else` over two separate `if` blocks**

The two independent `if (!worktreeIsValid)` / `if (worktreeIsValid)` blocks are mutually exclusive and can be collapsed into a single `if/else`, removing the superfluous second condition check and making the intent clearer.

```suggestion
        if (worktreeIsValid) {
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but +1

Comment on lines +187 to +218
if (!worktreeIsValid) {
log.warn(
`Worktree at ${worktreePath} is missing or not a git repository; skipping checkpoint capture`,
);
archivedTask.checkpointId = null;
}

if (worktreeIsValid) {
const actualBranch = await this.getCurrentBranchName(worktreePath);
if (actualBranch && actualBranch !== "HEAD") {
archivedTask.branchName = actualBranch;
}

await step(
async () => {
if (!archivedTask.checkpointId) {
throw new Error("checkpointId must be set for worktree mode");
}
await this.captureWorktreeCheckpoint(
folderPath,
worktreePath,
archivedTask.checkpointId,
);
},
async () => {
if (archivedTask.checkpointId) {
const git = createGitClient(folderPath);
await deleteCheckpoint(git, archivedTask.checkpointId);
}
},
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit but +1

@jonathanlab jonathanlab force-pushed the 04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally branch from a5618da to 45304f5 Compare April 29, 2026 10:26
@jonathanlab jonathanlab force-pushed the 04-28-fix_code_make_plan_approval_primary_option_match_user_s_prior_mode branch from c67bb56 to 271ba9c Compare April 29, 2026 10:26
Copy link
Copy Markdown
Contributor Author

jonathanlab commented Apr 29, 2026

Merge activity

@jonathanlab jonathanlab force-pushed the 04-28-fix_code_make_plan_approval_primary_option_match_user_s_prior_mode branch from 271ba9c to 1640b9e Compare April 29, 2026 11:06
@jonathanlab jonathanlab force-pushed the 04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally branch from 45304f5 to 7989298 Compare April 29, 2026 11:06
@jonathanlab jonathanlab changed the base branch from 04-28-fix_code_make_plan_approval_primary_option_match_user_s_prior_mode to graphite-base/1910 April 29, 2026 11:22
@jonathanlab jonathanlab changed the base branch from graphite-base/1910 to main April 29, 2026 11:28
When a workspace's worktree directory was deleted by an external process,
archiving failed because CaptureCheckpointSaga ran against a non-git path
and threw "fatal: not a git repository". The rollback then restored the
task, producing the flash-and-reappear behavior reported in #1798.

Now we probe the worktree with isGitRepository before checkpointing; if
the path is missing or invalid, we log the reason, set checkpointId to
null, and continue with the remaining cleanup steps. A null checkpointId
naturally causes unarchive to skip worktree restoration.

Generated-By: PostHog Code
Task-Id: eeaa1905-3fe5-423c-a853-08d75e78947b
@jonathanlab jonathanlab force-pushed the 04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally branch from 7989298 to e5704e9 Compare April 29, 2026 11:29
@jonathanlab jonathanlab merged commit 393b289 into main Apr 29, 2026
16 checks passed
@jonathanlab jonathanlab deleted the 04-28-fix_archive_skip_checkpoint_when_worktree_was_deleted_externally branch April 29, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Archive fails when worktree was deleted externally

2 participants